faet(sentry apps): Add context manager for sentry apps and impl for event webhooks #86136
Conversation
github-actions
bot
added
the
Scope: Backend
| logger.info("process_resource_change.event_missing_event", extra=extra) | ||
| with SentryAppInteractionEvent( | ||
| operation_type=SentryAppInteractionType.PREPARE_WEBHOOK, | ||
| event_type=f"process_resource_change.{sender}_{action}", |
There was a problem hiding this comment.
We dont have an 'event' string at this point so making something up
| lifecycle.record_failure(e) | ||
| return None | ||
| except (ApiHostError, ApiTimeoutError, RequestException, ClientError) as e: | ||
| lifecycle.record_halt(e) |
There was a problem hiding this comment.
ApiHostError & ApiTimeoutError are raised from send_and_save_webhook_request when the response is 503 or 504.
RequestExceptions occur when we get a Timeout or ConnectionError, these I also woudn't consider our server being at fault.
ClientErrors are anything <500 and also wouldn't be our fault, because it's the 3p responsibility to properly consume our requests.
We re raise since the TASK_OPTIONS already have the retry logic specified (i.e which errors to retry or ignore)
There was a problem hiding this comment.
JFYI that we will not encounter this block for NOT published apps (e.g internal & unpublished). As we only propagate up errors from send_and_save_webhook_request for published apps.
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #86136 +/- ##
===========================================
+ Coverage 68.06% 87.91% +19.84%
===========================================
Files 9700 9715 +15
Lines 550060 551151 +1091
Branches 21419 21419
===========================================
+ Hits 374406 484518 +110112
+ Misses 175270 66249 -109021
Partials 384 384 |
| MockResponseInstance = MockResponse({}, b"{}", "", True, 200, raiseStatusFalse, None) | ||
| MockResponse404 = MockResponse({}, b'{"bruh": "bruhhhhhhh"}', "", False, 404, raiseException, None) |
There was a problem hiding this comment.
need to be bytes for json.loads()
Per https://www.notion.so/sentry/Sentry-App-SLOs-19f8b10e4b5d805dacb9f80beccd1c65?pvs=4#1a68b10e4b5d807a80fbde13daf0ff7b
We're separating out the preparation and sending of webhooks.
Preparation
process_resource_change_boundSending
send_webhookssend_resource_change_webhooksend_and_save_webhook_requestWith the current implementation we will receive the following metrics:
sentry_app.prepare_webhook.{outcome}w/event_type: "process_resource_change.{sender}_{action}"senderwould beError,Groupactionwould be "created"sentry_app.send_webhook.{outcome}w/event_type:issue.createdorerror.created- potentially recorded 2 times (is this bad?)send_resource_change_webhookand send_and_save_webhook_request .send_resource_change_webhook&send_webhooksis p. minimal (also we have the logs to pinpoint failure place).